-
Notifications
You must be signed in to change notification settings - Fork 207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Map Clean Up #3198
Map Clean Up #3198
Conversation
…ion chunks if you have a non-lazy signal.
…ackends for the map function.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## RELEASE_next_major #3198 +/- ##
======================================================
- Coverage 81.30% 81.30% -0.01%
======================================================
Files 176 173 -3
Lines 24406 24207 -199
Branches 5681 5626 -55
======================================================
- Hits 19843 19681 -162
+ Misses 3258 3224 -34
+ Partials 1305 1302 -3
☔ View full report in Codecov by Sentry. |
…ion chunks if you have a non-lazy signal.
…ackends for the map function.
d7071fe
to
42fe611
Compare
42fe611
to
c22e028
Compare
2b84e1d
to
8b6fff1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CSSFrancis, I initially wanted to rebase to review and finish this PR, but got sidetracked with the documentation and spotted a bug with _map_all
. Can you please have a quick look to check that my changes are sensible?
Other than that it looks good to me.
|
||
Running computation on remote cluster can be done easily using ``dask_jobqueue`` | ||
|
||
.. code-block:: python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this example here, the example of the gallery needs to be run and I am not that it would be possible/easy on CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea I think maybe the distributed workflow would run but not the slurm workflow so you are right we should move it.
8b6fff1
to
13e7f7e
Compare
@ericpre I'll look at this a little later today or tomorrow. I've been pretty busy the last week or so and haven't been on top of some of these PRs. :) I'll have some time the next month to help with the major release. |
# Conflicts: # doc/user_guide/signal2d.rst # examples/lazy/distributed_backend.py # hyperspy/_signals/complex_signal.py # hyperspy/_signals/hologram_image.py # hyperspy/_signals/signal1d.py # hyperspy/_signals/signal2d.py # hyperspy/signal.py # upcoming_changes/3198.api.rst
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CSSFrancis, are you happy with my changes to this PR? If so, can you please merge it?
…dask-backends in bigdata.rst
@ericpre I'm going to check that the documentation builds this time and then I can merge this. Your changes look good to me! |
b22f2a5
into
hyperspy:RELEASE_next_major
Description of the change
This removes the
parallel
argument for themap
function.parallel
argumentmax_workers
tonum_workers
to be consistent with daskdask
backend_map_all
code path) - typically when it has theaxis
oraxes
keyword argument and the function argument are not navigation coordinate dependent.Progress of the PR
upcoming_changes
folder (seeupcoming_changes/README.rst
),readthedocs
doc build of this PR (link in github checks)